Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Stringable on storage attributes #1810

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

maxhelias
Copy link
Contributor

I didn't want to add the extends to the StorageAttributes interface to avoid breaking changes, but it can be done in the future if you want this feature

@SamMousa
Copy link

What's the use case for this?

You'll always need to iterate over the iterable since that won't be properly stringified.
So inside your loop you'd be able to do use echo or similar to dump a StorageAttributes to create strings in a very specific format that probably doesn't serve most use cases.

Can you provide some arguments for why this should be added to the library and not just live in some formatting code that lives in consuming projects?

@maxhelias
Copy link
Contributor Author

Improve the DX.
If I remember correctly, I have to array_map the iterator before I can render it as a console table. If the attributes are stringable, it will be automatic

@SamMousa
Copy link

If I remember correctly, I have to array_map the iterator before I can render it as a console table.

That's on a different level though, there is an iterable of StorageAttributes objects. In a console table you'd still want to loop over the iterable, or map it to an array.
Also, reasonably if you're drawing a table you'd be adding some padding and possibly show the type in a different column.

Where I'm going with this is that the specific formatting you suggest (or any other specific format) will always only be useful in very specific use cases.

I don't think this will affect the DX in any meaningfull way for 99% of the developers.

@maxhelias
Copy link
Contributor Author

The stringified format can indeed be revised, but making these objects Stringable is a common OOP technique that enhances flexibility. It’s useful for logging, debugging, and simple displays, allowing developers to handle these objects without needing to explicitly map them each time. This improves developer experience in many cases without imposing any specific usage.

@SamMousa
Copy link

SamMousa commented Oct 10, 2024

The stringified format can indeed be revised, but making these objects Stringable is a common OOP technique that enhances flexibility. It’s useful for logging, debugging, and simple displays, allowing developers to handle these objects without needing to explicitly map them each time. This improves developer experience in many cases without imposing any specific usage.

I fully agree with those points, there are downsides as well though:

  1. If you don't add it to the interface you cannot assume that a directory listing will always only contain these concrete implementations
  2. If you do add them to the interface it's a break in backwards compatibility and the benefit in DX for consumers is inversely correlated with the DX for implementers

Any ideas on how you'd tackle that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants